Skip to content

Use pm_arguments_end for function call#3634

Merged
kddnewton merged 1 commit into
ruby:mainfrom
mame:fix-CallNode-range-with-block-param
Sep 11, 2025
Merged

Use pm_arguments_end for function call#3634
kddnewton merged 1 commit into
ruby:mainfrom
mame:fix-CallNode-range-with-block-param

Conversation

@mame

@mame mame commented Sep 8, 2025

Copy link
Copy Markdown
Member

Previously, the location of CallNode was incorrect when it accepts a block parameter:

ruby -rprism -e 'pp Prism.parse("foo(&blk)").value.statements.body[0]']
@ CallNode (location: (1,0)-(1,8))  # <=== It should be (1,0)-(1,9)
├── flags: ∅
├── receiver: ∅
├── call_operator_loc: ∅
├── name: :foo
├── message_loc: (1,0)-(1,3) = "foo"
├── opening_loc: (1,3)-(1,4) = "("
├── arguments: ∅
├── closing_loc: (1,8)-(1,9) = ")"
*snip*

$ ruby -rprism -e 'pp Prism.parse("foo(&blk)").value.statements.body[0].slice'
"foo(&blk"

Note that the slice lacks the closing parenthesis.

Previously, the location of CallNode was incorrect when it accepts a
block parameter:

```
$ ruby -rprism -e 'pp Prism.parse("foo(&blk)").value.statements.body[0]']
@ CallNode (location: (1,0)-(1,8))  # <=== It should be (1,0)-(1,9)
├── flags: ∅
├── receiver: ∅
├── call_operator_loc: ∅
├── name: :foo
├── message_loc: (1,0)-(1,3) = "foo"
├── opening_loc: (1,3)-(1,4) = "("
├── arguments: ∅
├── closing_loc: (1,8)-(1,9) = ")"
*snip*

$ ruby -rprism -e 'pp Prism.parse("foo(&blk)").value.statements.body[0].slice'
"foo(&blk"
```

Note that the slice lacks the closing parenthesis.
@mame
mame force-pushed the fix-CallNode-range-with-block-param branch from e6ab4f1 to 3c22e6f Compare September 8, 2025 03:19
@mame

mame commented Sep 8, 2025

Copy link
Copy Markdown
Member Author

By the way, the AST design of BlockArgumentNode feels odd. Since the &blk is within the arguments as foo(&blk), it would be more natural for it to be a member of ArgumentsNode#arguments rather than a special position of CallNode#block.

@kddnewton

Copy link
Copy Markdown
Collaborator

By the way, the AST design of BlockArgumentNode feels odd. Since the &blk is within the arguments as foo(&blk), it would be more natural for it to be a member of ArgumentsNode#arguments rather than a special position of CallNode#block.

Originally that was the design, but we got feedback early on that it was much easier to reason about call nodes when all of the blocks were in the same place.

@kddnewton
kddnewton merged commit 603d651 into ruby:main Sep 11, 2025
60 checks passed
@mame
mame deleted the fix-CallNode-range-with-block-param branch September 12, 2025 07:51
@eregon

eregon commented Sep 12, 2025

Copy link
Copy Markdown
Member

Regarding BlockArgumentNode, see discussion in #1310 and linked issues/PRs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants